-
-
Notifications
You must be signed in to change notification settings - Fork 170
languages/formatters: allow multiple formatters #1103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
languages/formatters: allow multiple formatters #1103
Conversation
1fb7c25 to
61e7c8c
Compare
|
I think this will be easier if you cover all at once, because we will need to cover it before 0.8 anyway. |
|
aight bet |
906a500 to
6697e54
Compare
8b98f07 to
ba9ce8b
Compare
163eb0d to
2ca2b56
Compare
2ca2b56 to
d20abc1
Compare
d20abc1 to
393b777
Compare
NotAShelf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the good work.
I'm afraid I'll have to nit this a little, because I think we're inadvertently introducing some technical debt that will end up biting us in the ass later down the line.
- The functions with "deprecated" in their names are bothering me a little. Are we doing to get rid of those? If so, why not do that now since 0.8 is the big bad breaking change.
- The removal of "format" sections is not really good practice imo. We should strive to provide predictable and consistent APIs for the users, and I think removing two options does the opposite of contributing to this goal
- I left a few comments about unpacked sets, statix warns you about those and I'd rather get rid of them while we're already working on it.
0c0fe24 to
50ba157
Compare
2429336 to
06aa186
Compare
yea we're getting rid of them in the future, I just put up a little warning for now if you use the old syntax and convert it for you, just cuz I can and also cuz I hate breaking changes that don't say anything useful in logs/warning |
NotAShelf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to merge now and ask questions later, v0.8 has been delayed enough already.
|
...there are conflicts it's mergover |
06aa186 to
d319cfa
Compare
|
|
|
✅ Preview has been deleted successfully! |
Allow multiple formatters in
vim.languages.<lang>.format.type, deprecates use of non list typeAlso deprecates non-list values in
vim.languages.*.lsp.serverSanity Checking
nix fmt).#nix(default package).#maximal.#docs-html(manual, must build).#docs-linkcheck(optional, please build if adding links)x86_64-linuxaarch64-linuxx86_64-darwinaarch64-darwinAdd a 👍 reaction to pull requests you find important.